Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: add suffix string to docker container #1393

Merged
merged 5 commits into from
May 27, 2024

Conversation

jaeseung-bae
Copy link
Contributor

@jaeseung-bae jaeseung-bae commented May 23, 2024

Description

  • refresh docker container's bound volume which refer to current working directory
  • use current directory as a suffix for docker container to run it per their environment
    • Without refreshing docker container with bound volume location, It doesn't generate proto, swagger depending on the current working directory
    • For example
      • Does work for ~/Finschia/fisnchia-sdk
      • Doesn't work for ~/xxx/finschia-sdk
        • to work this, you should remove docker container first.

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml

@jaeseung-bae jaeseung-bae self-assigned this May 23, 2024
@jaeseung-bae jaeseung-bae marked this pull request as ready for review May 23, 2024 00:56
@jaeseung-bae jaeseung-bae requested a review from 170210 May 23, 2024 00:56
@jaeseung-bae jaeseung-bae changed the title build: refresh docker container's bound volume for proto and swagger … build: refresh docker container after use of proto and swagger generation May 23, 2024
Copy link
Member

@tkxkd0159 tkxkd0159 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then, it should be deleted if the containerProtoGen already exist instead of starting. Because this change assumes that the container will be regenerated every execution moment.

Copy link
Collaborator

@0Tech 0Tech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, we may just append the project name to the name of the container, which prevents the reuse by other projects.

@jaeseung-bae
Copy link
Contributor Author

Then, it should be deleted if the containerProtoGen already exist instead of starting.

Yes, It is. It should be deleted.

Or, we may just append the project name to the name of the container, which prevents the reuse by other projects.

Thanks for the comment, I'll try this one.

@jaeseung-bae jaeseung-bae requested review from 0Tech and tkxkd0159 May 24, 2024 05:36
Makefile Outdated Show resolved Hide resolved
@jaeseung-bae jaeseung-bae requested a review from 0Tech May 24, 2024 07:01
@jaeseung-bae jaeseung-bae changed the title build: refresh docker container after use of proto and swagger generation build: add suffix string to docker container May 27, 2024
@jaeseung-bae jaeseung-bae merged commit 0b92fe0 into Finschia:main May 27, 2024
33 checks passed
@jaeseung-bae jaeseung-bae deleted the build/swagger-gen branch May 27, 2024 04:45
mergify bot pushed a commit that referenced this pull request May 27, 2024
* build: refresh docker container's bound volume for proto and swagger generation

* Revert "build: refresh docker container's bound volume for proto and swagger generation"

This reverts commit 80d61b1.

* build: add current directory as suffix for docker container

* chore: update changelog

* chore: update pattern

(cherry picked from commit 0b92fe0)
mergify bot pushed a commit that referenced this pull request May 27, 2024
* build: refresh docker container's bound volume for proto and swagger generation

* Revert "build: refresh docker container's bound volume for proto and swagger generation"

This reverts commit 80d61b1.

* build: add current directory as suffix for docker container

* chore: update changelog

* chore: update pattern

(cherry picked from commit 0b92fe0)
zemyblue pushed a commit that referenced this pull request May 27, 2024
* build: refresh docker container's bound volume for proto and swagger generation

* Revert "build: refresh docker container's bound volume for proto and swagger generation"

This reverts commit 80d61b1.

* build: add current directory as suffix for docker container

* chore: update changelog

* chore: update pattern

(cherry picked from commit 0b92fe0)

Co-authored-by: jaeseung-bae <[email protected]>
jaeseung-bae added a commit that referenced this pull request May 27, 2024
* build: refresh docker container's bound volume for proto and swagger generation

* Revert "build: refresh docker container's bound volume for proto and swagger generation"

This reverts commit 80d61b1.

* build: add current directory as suffix for docker container

* chore: update changelog

* chore: update pattern

(cherry picked from commit 0b92fe0)

Co-authored-by: jaeseung-bae <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants